Skip to content

WMMA grouped conv fwd large tensor bias bnorm clamp #3595

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
wj-laskowski merged 4 commits into from
Jan 23, 2026
Merged

WMMA grouped conv fwd large tensor bias bnorm clamp #3595

wj-laskowski merged 4 commits into from
Jan 23, 2026

Conversation

@wj-laskowski
Copy link
Contributor

@wj-laskowski wj-laskowski commented Jan 16, 2026 •
edited
Loading

Proposed changes

Added bias bnorm clamp operation for WMMA conv fwd large tensor (FP16/BF16 data type and NHWGCxGKYXC layout).

Checklist

Please put an x into the boxes that apply. You can also fill these out after creating the PR. If you're not sure, please don't hesitate to ask.

  • I have added tests relevant to the introduced functionality, and the unit tests are passing locally
  • I have added the test to REGRESSION_TESTS list defined at the top of CMakeLists.txt in tests/CMakeLists.txt, IF the test takes more than 30 seconds to run.
  • I have added inline documentation which enables the maintainers with understanding the motivation
  • I have removed the stale documentation which is no longer relevant after this pull request
  • (If this change is user-facing) I have added release notes which provide the end users with a brief summary of the improvement from this pull request
  • I have run clang-format on all changed files
  • Any dependent changes have been merged

Discussion

If this is a relatively large or complex change, feel free to start a discussion by explaining why you chose the solution you did and what alternatives you considered

@wj-laskowski wj-laskowski force-pushed the streamhpc/grouped-conv-fwd-wmma-large-tensor-bias_bnorm_clamp branch from 7b0341d to b5c541f Compare January 19, 2026 09:35
@wj-laskowski wj-laskowski changed the title Streamhpc/grouped conv fwd wmma large tensor bias bnorm clamp WMMA grouped conv fwd large tensor bias bnorm clamp Jan 19, 2026
@wj-laskowski wj-laskowski marked this pull request as ready for review January 19, 2026 12:58
@krithalith
Copy link
Contributor

krithalith commented Jan 20, 2026

Looks good overall! I have some comments:

  1. They wanted only a single generic instance per instance list for (Large Tensor) bias bnorm clamp. You can just make two new instance lists that only contain one generic instance and use them only for bias bnorm clamp. We did the same for the non-large version. Please first find an actual generic instance, because it seems like the currents instance lists do not contain one. You can probably adapt them from the XDL Large Tensor instance lists. They should have all the scalarPerVector values equal to 1.
  2. Are the current bias bnorm clamp tests sufficient for testing the Large implementation? I.e. are the tensors large enough to actually cause splitting? If not it might be useful to add a "Large Cases" test like for the other flavors.
  3. Did you check if the Large Tensor implementation is actually run and can support all the test cases (especially check after reducing instance lists to generic only)?

@krithalith krithalith self-requested a review January 20, 2026 09:53
@wj-laskowski wj-laskowski mentioned this pull request Jan 21, 2026
Merged
7 tasks
@ApoorvaKalyani ApoorvaKalyani force-pushed the streamhpc/conv_fwd_wmma_bias_bnorm_clamp branch from 23c6688 to 7306d8b Compare January 21, 2026 10:51
krithalith
krithalith previously approved these changes Jan 21, 2026
View reviewed changes
@bartekxk bartekxk requested a review from Copilot January 21, 2026 11:21
bartekxk
bartekxk previously approved these changes Jan 21, 2026
View reviewed changes
Copy link
Contributor

@bartekxk bartekxk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm please rebase

Copilot AI reviewed Jan 21, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request adds WMMA (Wave Matrix Multiply Accumulate) kernel support for grouped convolution forward operations with bias, batch normalization, and clamp activation for large tensors. The implementation targets FP16 and BF16 data types with NHWGC/GKYXC/NHWGK tensor layouts.

Changes:

  • Added WMMA kernel instances for 2D and 3D grouped convolutions with bias+bnorm+clamp operations
  • Updated test infrastructure to enable bias+bnorm+clamp tests on gfx9, gfx11, and gfx12 GPU targets
  • Modified initialization ranges in the profiler to ensure monotone operations, improving numerical stability on RDNA3 architectures

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/grouped_convnd_fwd_activation/test_grouped_convnd_fwd_gk_bias_bnorm_clamp.cpp Updated function call from bias_clamp to bias_bnorm_clamp
test/grouped_convnd_fwd_activation/test_grouped_convnd_fwd_bias_bnorm_clamp.cpp Updated function call from bias_clamp to bias_bnorm_clamp
test/grouped_convnd_fwd_activation/CMakeLists.txt Moved bias_bnorm_clamp tests to gfx9/11/12 target section
test/CMakeLists.txt Added test to regression test list
profiler/src/profile_grouped_conv_fwd_bias_bnorm_clamp.cpp New profiler entry point for bias_bnorm_clamp operation
profiler/src/CMakeLists.txt Added profiler source and device instances to build
profiler/include/profiler/profile_grouped_conv_fwd_bias_bnorm_clamp_impl.hpp Changed initialization ranges to monotone values for numerical stability
library/src/tensor_operation_instance/gpu/grouped_conv3d_fwd_bias_bnorm_clamp/wmma/large_tensor/*.cpp WMMA large tensor instances for 3D conv (f16/bf16)
library/src/tensor_operation_instance/gpu/grouped_conv3d_fwd_bias_bnorm_clamp/wmma/*.cpp WMMA instances for 3D conv (f16/bf16)
library/src/tensor_operation_instance/gpu/grouped_conv3d_fwd_bias_bnorm_clamp/CMakeLists.txt Updated build configuration to include WMMA instances
library/src/tensor_operation_instance/gpu/grouped_conv2d_fwd_bias_bnorm_clamp/wmma/large_tensor/*.cpp WMMA large tensor instances for 2D conv (f16/bf16)
library/src/tensor_operation_instance/gpu/grouped_conv2d_fwd_bias_bnorm_clamp/wmma/*.cpp WMMA instances for 2D conv (f16/bf16)
library/src/tensor_operation_instance/gpu/grouped_conv2d_fwd_bias_bnorm_clamp/CMakeLists.txt Updated build configuration to include WMMA instances
library/include/ck/library/tensor_operation_instance/gpu/grouped_convolution_forward_bias_bnorm_clamp_wmma_cshufflev3.inc Forward declarations for WMMA instance functions
library/include/ck/library/tensor_operation_instance/gpu/grouped_convolution_forward_bias_bnorm_clamp.hpp Factory integration for WMMA instances
library/include/ck/library/tensor_operation_instance/gpu/grouped_conv_fwd/device_grouped_conv_fwd_wmma_cshufflev3_large_tensor_instance.hpp Generic instance templates for WMMA large tensor
library/include/ck/library/tensor_operation_instance/gpu/grouped_conv_fwd/device_grouped_conv_fwd_wmma_cshufflev3_instance.hpp Generic instance templates and type aliases
include/ck/utility/array.hpp Added Emplace method for in-place construction
include/ck/tensor_operation/gpu/device/impl/device_grouped_conv_fwd_multiple_d_wmma_cshuffle_v3_large_tensor.hpp Refactored to use Emplace and added NumDTensor guards

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@wj-laskowski wj-laskowski force-pushed the streamhpc/grouped-conv-fwd-wmma-large-tensor-bias_bnorm_clamp branch from 35f342c to 427b259 Compare January 21, 2026 12:13
Base automatically changed from streamhpc/conv_fwd_wmma_bias_bnorm_clamp to develop January 22, 2026 08:54
@ApoorvaKalyani ApoorvaKalyani dismissed stale reviews from bartekxk and krithalith January 22, 2026 08:54

The base branch was changed.

@wj-laskowski
Added bias_bnorm_clamp for WMMA conv fwd large tensor.
24b6bd7 
Following operations are added for FP16/BF16 data type and NHWGCxGKYXC layout.
- grouped_conv2d_fwd_bias_bnorm_clamp
- grouped_conv3d_fwd_bias_bnorm_clamp
@wj-laskowski
changed strategy to handle GemmArgs array
2052f4b
@wj-laskowski
Adding generic instance
1aef63f
@wj-laskowski
fixed last nits from reviewers and copilot
41ca771
@wj-laskowski wj-laskowski force-pushed the streamhpc/grouped-conv-fwd-wmma-large-tensor-bias_bnorm_clamp branch from 427b259 to 41ca771 Compare January 22, 2026 09:11
@wj-laskowski wj-laskowski requested a review from bartekxk January 23, 2026 07:26
@wj-laskowski wj-laskowski merged commit 2e08a7e into develop Jan 23, 2026
22 checks passed
@wj-laskowski wj-laskowski deleted the streamhpc/grouped-conv-fwd-wmma-large-tensor-bias_bnorm_clamp branch January 23, 2026 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@bartekxk bartekxk bartekxk approved these changes

@krithalith krithalith krithalith left review comments

@illsilin illsilin Awaiting requested review from illsilin illsilin is a code owner

@carlushuang carlushuang Awaiting requested review from carlushuang carlushuang is a code owner

@qianfengz qianfengz Awaiting requested review from qianfengz qianfengz is a code owner

@aosewski aosewski Awaiting requested review from aosewski aosewski is a code owner

@poyenc poyenc Awaiting requested review from poyenc poyenc is a code owner

@geyyer geyyer Awaiting requested review from geyyer geyyer is a code owner

@andriy-ca andriy-ca Awaiting requested review from andriy-ca andriy-ca is a code owner

@afagaj afagaj Awaiting requested review from afagaj afagaj is a code owner

@asleepzzz asleepzzz Awaiting requested review from asleepzzz asleepzzz is a code owner

@ThomasNing ThomasNing Awaiting requested review from ThomasNing ThomasNing is a code owner

@coderfeli coderfeli Awaiting requested review from coderfeli coderfeli is a code owner

@cgmillette cgmillette Awaiting requested review from cgmillette cgmillette is a code owner

@shumway shumway Awaiting requested review from shumway shumway is a code owner

@vidyasagar-amd vidyasagar-amd Awaiting requested review from vidyasagar-amd vidyasagar-amd is a code owner

@tenpercent tenpercent Awaiting requested review from tenpercent tenpercent is a code owner

@vpietila-amd vpietila-amd Awaiting requested review from vpietila-amd vpietila-amd is a code owner

@Snektron Snektron Awaiting requested review from Snektron Snektron is a code owner

+1 more reviewer

@chris-tsiaousis-hpc chris-tsiaousis-hpc chris-tsiaousis-hpc left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@bartekxk bartekxk

Labels

organization: streamhpc

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@wj-laskowski @krithalith @bartekxk @chris-tsiaousis-hpc